Skip to content

pin_attempts_exceeded in cards#179

Merged
rogelioLpz merged 3 commits intomainfrom
fields-cards
Aug 2, 2021
Merged

pin_attempts_exceeded in cards#179
rogelioLpz merged 3 commits intomainfrom
fields-cards

Conversation

@rogelioLpz
Copy link
Copy Markdown
Member

@rogelioLpz rogelioLpz commented Jul 30, 2021

closes #178

@rogelioLpz rogelioLpz added the enhancement New feature or request label Jul 30, 2021
@rogelioLpz rogelioLpz self-assigned this Jul 30, 2021
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 30, 2021 •
edited
Loading

Codecov Report

Merging #179 (23759d6) into main (aee295c) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #179   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           27        27           
  Lines          652       657    +5     
=========================================
+ Hits           652       657    +5
Flag Coverage Δ
unittests 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cuenca/resources/cards.py 100.00% <100.00%> (ø)
cuenca/version.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aee295c...23759d6. Read the comment docs.

@rogelioLpz rogelioLpz requested a review from alexviquez July 30, 2021 19:03
@rogelioLpz rogelioLpz marked this pull request as ready for review July 30, 2021 19:03
@rogelioLpz rogelioLpz requested a review from matin as a code owner July 30, 2021 19:03
alexviquez
alexviquez suggested changes Jul 30, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@alexviquez alexviquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creo que lo de los pin_attemps debería funcionar como actualmente los card_activation_attemps, ya que como lo diseñaste, sobrescribirías un valor todo el tiempo, cuando en realidad podrías crear objeto por intento fallido y tener un registro incluso a que transacción fue la que hizo que fallara

@rogelioLpz
Copy link
Copy Markdown
Member Author

rogelioLpz commented Jul 30, 2021

Creo que lo de los pin_attemps debería funcionar como actualmente los card_activation_attemps, ya que como lo diseñaste, sobrescribirías un valor todo el tiempo, cuando en realidad podrías crear objeto por intento fallido y tener un registro incluso a que transacción fue la que hizo que fallara

Si se lleva registro en knox.

@rogelioLpz rogelioLpz requested a review from alexviquez August 2, 2021 20:57
alexviquez
alexviquez approved these changes Aug 2, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@alexviquez alexviquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Va, no estoy tan de acuerdo que sea un campo pero comprendo que es necesario para performance

@rogelioLpz rogelioLpz merged commit 1edf929 into main Aug 2, 2021
@rogelioLpz rogelioLpz deleted the fields-cards branch August 2, 2021 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@matin matin Awaiting requested review from matin

1 more reviewer

@alexviquez alexviquez alexviquez approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@rogelioLpz rogelioLpz

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add pin_attemps in resource card

2 participants

@rogelioLpz @alexviquez